Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix ThreadLocal tracking behavior #56956

Merged
merged 1 commit into from
Aug 10, 2021
Merged

Fix ThreadLocal tracking behavior #56956

merged 1 commit into from
Aug 10, 2021

Conversation

davidwrighton
Copy link
Member

  • Before this change the trackAllValues behavior for ThreadLocal was defined by the first instance of thread local to have its value set on the thread
    • This could lead to unpredictable memory leaks (where the value was improperly tracked even though it wasn't supposed to be) This reproduces as a memory leak with no other observable behavior
    • Or data loss, where the Values collection was missing entries.
  • Change the model so that ThreadLocal trackAllValues behavior is properly defined by the exact ThreadLocal instance in use
  • Implement by keeping track of the track all changes behavior within the IdManager

Fixes #55796

@davidwrighton
Fix ThreadLocal tracking behavior
c1bee3c 
- Before this change the trackAllValues behavior for ThreadLocal<SomeParticularType> was defined by the first instance of thread local to have its value set on the thread
  - This could lead to unpredictable memory leaks (where the value was improperly tracked even though it wasn't supposed to be) This reproduces as a memory leak with no other observable behavior
  - Or data loss, where the Values collection was missing entries.
- Change the model so that ThreadLocal<T> trackAllValues behavior is properly defined by the exact ThreadLocal<T> instance in use
- Implement by keeping track of the track all changes behavior within the IdManager

Fixes dotnet#55796
@ghost
Copy link

ghost commented Aug 6, 2021

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.

Issue Details
  • Before this change the trackAllValues behavior for ThreadLocal was defined by the first instance of thread local to have its value set on the thread
    • This could lead to unpredictable memory leaks (where the value was improperly tracked even though it wasn't supposed to be) This reproduces as a memory leak with no other observable behavior
    • Or data loss, where the Values collection was missing entries.
  • Change the model so that ThreadLocal trackAllValues behavior is properly defined by the exact ThreadLocal instance in use
  • Implement by keeping track of the track all changes behavior within the IdManager

Fixes #55796

Author: davidwrighton
Assignees: -
Labels:

area-System.Threading
Milestone: -

kouvel
kouvel approved these changes Aug 10, 2021
View reviewed changes
Copy link
Member

@kouvel kouvel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!

@davidwrighton davidwrighton merged commit fc2c089 into dotnet:main Aug 10, 2021
This was referenced Aug 17, 2021
Closed
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Sep 9, 2021
@davidwrighton davidwrighton deleted the fix_threadlocal_tracking branch April 13, 2023 18:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kouvel kouvel kouvel approved these changes

@stephentoub stephentoub Awaiting requested review from stephentoub

Assignees
No one assigned
Labels
area-System.Threading
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ThreadLocals of the same type T can interfere with each other's trackAllValues
2 participants
@davidwrighton @kouvel